fix(capping): reup capping on connection creation #2674
Conversation
WalkthroughThe changes involve updates to error handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Middleware
participant Server
User->>Frontend: Create Integration
Frontend->>Middleware: Validate Integration
Middleware-->>Frontend: Integration Validated
Frontend->>Server: Setup Connection
Server-->>Frontend: Connection Setup Successful
Frontend-->>User: Integration Created
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- packages/frontend/lib/index.ts (7 hunks)
- packages/server/lib/hooks/hooks.ts (1 hunks)
- packages/server/lib/middleware/resource-capping.middleware.ts (1 hunks)
- packages/server/lib/routes.ts (3 hunks)
- packages/server/lib/utils/asyncWrapper.ts (1 hunks)
- packages/types/lib/api.ts (1 hunks)
- packages/types/lib/flow/http.api.ts (1 hunks)
- packages/webapp/src/pages/Connection/Create.tsx (1 hunks)
Additional comments not posted (18)
packages/server/lib/middleware/resource-capping.middleware.ts (4)
2-2: LGTM!The code changes are approved.
5-6: LGTM!The code changes are approved.
7-16: LGTM!The code changes are approved.
19-20: LGTM!The code changes are approved.
packages/types/lib/flow/http.api.ts (1)
31-31: LGTM! The code change aligns with the PR objectives.The removal of the
ApiError<'resource_capped'>variant from theErrorproperty of thePostPreBuiltDeploytype is consistent with the PR objectives and the AI-generated summary. This change indicates that the API will no longer handle or report errors related to resource capping in the context of pre-built deployments.Clients of the API should be aware that they will no longer receive specific feedback regarding resource limitations during deployment processes. They may need to update their error handling logic accordingly.
The remaining error types in the union type (
ApiError<'unknown_provider'>,ApiError<'failed_to_deploy', Error[]>, andApiError<'unknown_flow'>) are still relevant and appropriate for thePostPreBuiltDeployendpoint.packages/types/lib/api.ts (1)
23-24: LGTM!The addition of the
ApiError<'resource_capped'>error type to theResDefaultErrorsunion is a valuable enhancement to the API's error handling capabilities. It allows the system to provide more specific feedback to clients when a resource has reached its capacity limit.The change seamlessly integrates with the existing error types without affecting their handling, enriching the API's response options while maintaining the overall structure of the
ResDefaultErrorstype.The code changes are approved.
packages/server/lib/hooks/hooks.ts (2)
54-56: LGTM!The addition of the early return condition enhances the function's robustness and efficiency by explicitly handling the case where
scriptConfigsis empty. This avoids unnecessary iterations over an empty array.
58-59: Also applies to: 61-66packages/server/lib/routes.ts (5)
29-29: LGTM!The import of
RequestHandlertype fromexpressis approved.
88-88: LGTM!The type annotation for the
apiAutharray is approved.
89-93: LGTM!The type annotation for the
adminAutharray is approved.
94-96: LGTM!The type annotations for the
apiPublicAuthandwebAutharrays are approved.
20-20: Verify the impact of replacingauthCheckwithresourceCappingmiddleware.Ensure that the
resourceCappingmiddleware provides the necessary authentication and authorization checks for the API public authentication flow.Run the following script to verify the middleware usage:
Verification successful
Verified: The
resourceCappingmiddleware is correctly integrated into the API public authentication flow.The
resourceCappingmiddleware is used in theapiPublicAutharray inpackages/server/lib/routes.ts, confirming its role in the authentication process. It is defined inpackages/server/lib/middleware/resource-capping.middleware.ts, indicating its functionality is implemented as expected.
packages/server/lib/routes.ts: Usage inapiPublicAutharray.packages/server/lib/middleware/resource-capping.middleware.ts: Middleware definition.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `resourceCapping` middleware in the API public authentication flow. # Test: Search for the middleware usage in the file. Expect: Only occurrences in the `apiPublicAuth` array. rg --type typescript $'resourceCapping'Length of output: 73
Script:
#!/bin/bash # Description: Verify the usage of `resourceCapping` middleware in the API public authentication flow. # Test: Search for the middleware usage in the file. Expect: Only occurrences in the `apiPublicAuth` array. rg --type ts 'resourceCapping'Length of output: 499
packages/frontend/lib/index.ts (4)
100-100: LGTM!The code changes are approved. The updated error handling mechanism enhances the specificity of the error information being passed to the
AuthError, potentially allowing for more precise error handling and debugging.
312-312: LGTM!The code changes are approved. The updated error handling mechanism is consistent with the previous code segment and enhances the specificity of the error information being passed to the
AuthError.
333-333: LGTM!The code changes are approved. The updated error handling mechanism is consistent with the previous code segments and enhances the specificity of the error information being passed to the
AuthError.Also applies to: 354-354
375-375: LGTM!The code changes are approved. The updated error handling mechanism is consistent with the previous code segments and enhances the specificity of the error information being passed to the
AuthError.Also applies to: 396-396, 417-417
packages/webapp/src/pages/Connection/Create.tsx (1)
242-242: LGTM!Resetting the
serverErrorMessagestate to an empty string when an existingintegrationis detected is a good practice. It clears any previous server error messages before setting up the connection configuration parameters and authentication mode based on the current integration, improving the user experience by preventing stale error messages from persisting when a valid integration is loaded.
packages/server/lib/hooks/hooks.ts
Outdated
| @@ -51,18 +51,19 @@ export const connectionCreationStartCapCheck = async ({ | |||
| } | |||
|
|
|||
| const scriptConfigs = await getSyncConfigsWithConnections(providerConfigKey, environmentId); | |||
| if (scriptConfigs.length <= 0) { | |||
| return false; | |||
| } | |||
There was a problem hiding this comment.
not sure this check is useful. If empty the for loop below will immediately exit
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/server/lib/hooks/hooks.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/server/lib/hooks/hooks.ts
Describe your changes
Fixes https://linear.app/nango/issue/NAN-1638/capping-on-connection-creation-is-not-working-anymore
Reup capping
Fix error handling
Unfortunately, we have mixed public and internal API, during error handling rework it changed the API response for both :/
It was not broken but just displaying empty error
UI: fix error not resetting when changing integration
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores